Skip to content

fix: write tabs session file atomically to survive unclean shutdown - #2356

Open
jorgeecardona wants to merge 1 commit into
Guake:masterfrom
jorgeecardona:crash-safe-save-tabs
Open

jorgeecardona wants to merge 1 commit into
Guake:masterfrom
jorgeecardona:crash-safe-save-tabs

Conversation

@jorgeecardona

@jorgeecardona jorgeecardona commented Oct 10, 2026 •

Copy link
Copy Markdown
Collaborator

A crash or power loss while saving tabs could leave a half-written session.json, and restore then discarded it.

  • Write to session.json.tmp, fsync, os.replace, then fsync the directory.
  • Test checks the result and both fsyncs.

Copilot AI balanced review requested due to automatic review settings October 10, 2026 15:58

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Temporary-path construction breaks custom filenames containing directory components, and the test does not exercise interrupted writes.

2 open findings
What changed in this PR

Makes tab-session persistence crash-safe through atomic replacement and filesystem syncing.

Changes:

  • Writes sessions through a temporary file and atomically replaces the destination.
  • Adds atomic-save coverage and a release note.
File Description
guake/​guake_app.py Implements atomic, durable session saving.
guake/​tests/​test_guake.py Tests the new save path.
releasenotes/​notes/​crash-safe-save-tabs-adf00755bead6ea5.yaml Documents the fix.

🧠 Review effort: Balanced


💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread guake/guake_app.py Outdated
Comment thread guake/tests/test_guake.py
save_tabs truncated session.json in place, so a crash or power loss
mid-write left a half-written file that restore_tabs discarded as
broken.

Write to a sibling .tmp file, flush and fsync it, os.replace it onto
session.json, then fsync the directory so the rename is durable.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants